Skip to content
This repository has been archived by the owner on Dec 1, 2022. It is now read-only.

Add support to compare different numerical types in LOOKUP WHERE clause #784

Merged
merged 10 commits into from
Mar 9, 2021

Conversation

Aiee
Copy link
Contributor

@Aiee Aiee commented Mar 4, 2021

  1. Refactor the implementation of Fix indexScan rule when compare INT with Double #777
  2. Add support to LOOKUP queries like LOOKUP ON player WHERE player.age >= 40.5 YIELD player.age AS Age
  3. The main idea is to check/cast different numerical values in the validation stage so that optimizer and indexScan rule can keep the current logic.

TODOs:

  1. Add the same capability to MATCH
  2. extract the typecast logic as a method if we decide to support more cross-type comparisons.

@Aiee Aiee added the ready-for-testing PR: ready for the CI test label Mar 4, 2021
@Aiee Aiee force-pushed the fix-lookup-filter branch from 29d66db to 5209789 Compare March 4, 2021 11:12
@bright-starry-sky
Copy link
Contributor

Good job.

@Aiee Aiee requested a review from a team March 5, 2021 07:50
@Aiee Aiee force-pushed the fix-lookup-filter branch from c8be1f8 to 562d73e Compare March 5, 2021 08:33
src/validator/LookupValidator.cpp Outdated Show resolved Hide resolved
src/validator/LookupValidator.cpp Outdated Show resolved Hide resolved
@Aiee Aiee force-pushed the fix-lookup-filter branch from 7c2111a to 1dbbcba Compare March 5, 2021 10:18
@Aiee Aiee requested a review from a team March 8, 2021 07:11
Copy link
Contributor

@bright-starry-sky bright-starry-sky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now, let ours handle other type later.

@yixinglu yixinglu merged commit d04aae9 into vesoft-inc:master Mar 9, 2021
@Aiee Aiee deleted the fix-lookup-filter branch March 9, 2021 08:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants